The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
rivetkit-serverless	Error			Dec 11, 2025 1:10am

3 Skipped Deployments

Project	Deployment	Preview	Comments	Updated (UTC)
rivet-cloud	Ignored			Dec 11, 2025 1:10am
rivet-inspector	Ignored	Preview		Dec 11, 2025 1:10am
rivet-site	Ignored	Preview		Dec 11, 2025 1:10am

• chore(engine): test actors runner kv #3625
• chore(engine): test actor alarms #3624
• feat(engine): actors lifecycle testing #3623
• chore: setup rivet-engine api tests #3540 👈 (View in Graphite)
• main

How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

This stack of pull requests is managed by Graphite. Learn more about stacking.

PR Review: Setup rivet-engine API Tests

This PR reorganizes and enhances the test suite for rivet-engine's API layer. Overall, this is a solid refactoring that improves test organization and adds better API testing infrastructure. Here's my detailed feedback:

✅ Strengths

1. Excellent Code Deduplication

The creation of shared type definitions in api-types package is a great architectural improvement:

• actors/delete.rs, runners/list_names.rs now shared between peer and public APIs
• Eliminates duplicate type definitions across API layers
• Adds proper Deserialize implementations for better testability

2. Improved Test Organization

• Splitting tests into api_* prefix clearly distinguishes API-level tests from unit tests
• New test helper modules (common/api/public.rs, common/api/peer.rs) provide clean abstractions
• Better separation of concerns with dedicated test utilities

3. Comprehensive Test Coverage

The new test files demonstrate thorough coverage:

• api_actors_list.rs: 1798 additions covering pagination, filtering, edge cases
• api_actors_delete.rs: 481 additions for deletion scenarios
• api_namespaces_create.rs, api_runner_configs_*: Good coverage of infrastructure

4. Better Use of Test Infrastructure

• Proper use of rivet_pools::reqwest::client() (follows CLAUDE.md optimization guidelines)
• Consolidated test helpers reduce boilerplate

🔍 Issues & Concerns

Critical: Cursor Pagination Logic in actors/list.rs (lines 112-120)

This logic applies cursor filtering when fetching by actor IDs, but cursor-based pagination doesn't make semantic sense here because you're fetching specific items by ID, not paginating through a list.

Problem: When fetching by IDs specifically, the user is asking for specific actors by ID. The cursor comparison uses less than, meaning we want actors created before the cursor timestamp. However, the fanout path (line 186-204) doesn't apply cursor filtering at all—it relies on peer handlers to do it. This creates an inconsistency.

Recommendation: Either remove cursor filtering for the actor_ids path (users fetching by ID shouldn't need pagination) or document why cursor pagination is needed when fetching specific IDs.

Location: engine/packages/api-public/src/actors/list.rs:112-120

Medium: Missing Error Context in DeletePath

In api-types/src/actors/delete.rs:12-16, DeletePath is missing Debug and documentation. While minor, this impacts debugging (can't easily print the path in logs) and API documentation clarity.

Recommendation: Add derive(Debug) and a doc comment explaining this is the path parameter for actor deletion.

Medium: Inconsistent Deserialize Traits

In api-peer/src/internal.rs, you added Deserialize to several response types (lines 13, 41, 49, etc.): CachePurgeResponse, SetTracingConfigResponse, etc.

Question: Why do response types need Deserialize? Response types are typically only serialized (for sending to clients), not deserialized.

Possible Reason: You might be using these types in tests to parse responses. If so, this is acceptable for test convenience. Consider adding a comment explaining this is for test deserialization or use a feature flag like cfg_attr(test, derive(Deserialize)).

Location: engine/packages/api-peer/src/internal.rs:13,41,49,100,109,129,137

Low: Missing Validation Test Cases

While test coverage is comprehensive, I noticed some edge cases might not be covered:

1. Cursor validation: What happens if cursor is not a valid i64? (The code handles this at line 114 with .parse().context(), which is good, but no test validates the error response)

2. Limit boundary: What happens with limit=0? limit=10000? The code uses unwrap_or(100) so 0 would be respected, but is that intentional?

3. Namespace validation: The new CreateRequest in api-peer/src/namespaces.rs:72-73 made fields public. Are there tests validating empty strings, max length, special characters?

🎯 Code Quality Observations

Good Practices ✅

• Consistent error handling: Using anyhow::Context properly
• Structured logging: Following CLAUDE.md guidelines with tracing parameters
• No glob imports: Good adherence to "Do not glob import from anyhow" guideline

Suggestions

1. Add inline comments for complex logic: The cursor filtering logic and fanout behavior in list.rs would benefit from inline comments explaining why cursor is applied differently in different paths.

2. Test naming: The new api_actors_list.rs file is well-structured, but some test names could be more descriptive.

3. Test documentation: Consider adding module-level doc comments in test files explaining what aspect of the API each file tests.

🔒 Security Considerations

No critical security issues found, but worth noting:

1. Input validation: The code properly validates actor_ids count (max 32) at line 86-93 in list.rs. ✅
2. SQL injection: Not applicable (using typed IDs, not raw SQL) ✅
3. DoS via large inputs: The actor_ids limit prevents excessive fanout. Good. ✅

📊 Performance Considerations

Potential Issue: Fanout Efficiency

In api-public/src/actors/list.rs:188-197, when using the fanout path, you're fanning out the entire query object to all datacenters, then truncating to limit afterward (line 204).

Performance Impact: If you have 10 datacenters and limit=100, each DC might return up to 100 actors, giving you up to 1000 actors to sort and truncate. This is inefficient but probably acceptable for typical use cases.

Recommendation: Document this behavior in the function comments (you already have good comments about round trips, this would complement them).

🧪 Test Coverage Assessment

Overall: Excellent

• ✅ Positive cases well covered
• ✅ Edge cases (empty results, pagination boundaries) tested
• ✅ Error cases tested (invalid namespace, etc.)
• ⚠️ Could add more validation error tests (see "Missing Validation Test Cases" above)

Summary & Recommendations

Overall Assessment: ✅ Approve with minor suggestions

This PR significantly improves the test infrastructure and organization. The critical issue with cursor pagination should be addressed, but it's a logic clarification rather than a breaking bug. The deduplication of types and creation of proper test utilities are excellent architectural improvements.

Great work on the comprehensive test coverage and following the project's conventions! 🎉

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/cloudflare-workers@3540

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/db@3540

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/framework-base@3540

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/next-js@3540

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/react@3540

pnpm add https://pkg.pr.new/rivet-dev/rivet/rivetkit@3540

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sql-loader@3540

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner@3540

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner-protocol@3540

commit: ebaa134